[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector by samsonasik · Pull Request #5039 · rectorphp/rector-src

samsonasik · 2023-09-17T23:05:03Z

@staabm this is to handle fallback from param on return new object:

final class FallbackFromParam
{
    public function action(SomeResponse $someResponse)
    {
        if (rand(0, 1)) {
            return new SomeResponse();
        }

        return $someResponse;
    }
}

…ewRector

samsonasik · 2023-09-17T23:06:26Z

implemented 🎉

samsonasik · 2023-09-17T23:19:09Z

@TomasVotruba the phpstan notice seems due to new phpstan-extensions release that cause invalid check on realpath() which can be false:

------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  packages/Caching/Detector/ChangedFilesDetector.php:90
 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  - '#Strict comparison using \=\=\= between string and false will always evaluate to false#'
 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------

see symplify/phpstan-extensions@b2b82e1#r127599628

samsonasik · 2023-09-17T23:28:21Z

@TomasVotruba I created PR to symplify/phpstan-extensions to revert realpath:

Remove realpath on NativeFunctionDynamicFunctionReturnTypeExtension symplify/phpstan-extensions#7

staabm · 2023-09-18T05:18:59Z

+
+final class FallbackFromParamSelf
+{
+    public function action(self $obj): \Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture\FallbackFromParamSelf


Do we need another test for a static and/or parent typed param?

static already error on the first place :)

https://3v4l.org/cEZik

multiple return already skipped at

rector-src/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromReturnNewRector.php

Lines 185 to 186 in c956f86

$returnType = $this->returnTypeInferer->inferFunctionLike($node);

if ($returnType instanceof UnionType) {

samsonasik · 2023-09-18T06:00:56Z

I updated to use @var string|false and check with more verbose is_dir() && is_file() in various locations based on @TomasVotruba suggestion at

symplify/phpstan-extensions@b2b82e1#r127599628

samsonasik · 2023-09-18T06:02:37Z

All checks have passed 🎉 @TomasVotruba I think it is ready.

staabm · 2023-09-18T06:04:09Z

+            // not file
+            && ! is_file($skipRule)
            // a Rector end
            && str_ends_with($skipRule, 'Rector')


We should do this cheap chrck earlier

updated 8fae378

samsonasik · 2023-09-18T06:12:33Z

All checks have passed 🎉 @TomasVotruba I am merging it to have faster feedback to test ;)

TomasVotruba · 2023-09-18T06:15:19Z

+            /** @var string|false $setRealpath */
+            $setRealpath = realpath($setFile);
+            $setRealpath = (string) $setRealpath;
            $relativeFilePath = Strings::after($setRealpath, getcwd() . '/');


This will be always string, as we use existing set paths:

$setRealpath = realpath($setFile); $relativeFilePath = Strings::after($setRealpath, getcwd() . '/');

sure, I updated at :

Use realpath() as string on MissingInSetCommand #5040

Thanks 👏

the php process might still not be able to read this file (e.g. because of filesystem persmissions) and return false?

@staabm that's what I thought, but since tools/ directory is not shipped to rector/rector, I guess it safe for user, the concern is probably when directory/set removed in the future and not detected.

The better solution is probably use:

Assert::string($setRealpath);

so it will be detected early when path removed/renamed.

staabm · 2023-09-18T06:16:42Z

Thanks 🙏

[TypeDeclaration] Handle fallback from param on ReturnTypeFromReturnN…

be8b2eb

…ewRector

samsonasik requested a review from TomasVotruba as a code owner September 17, 2023 23:05

implemented 🎉

7594001

actions-user and others added 3 commits September 17, 2023 23:07

[ci-review] Rector Rectify

2a154c6

rename fixture

e69bc6e

rename fixture

f93af9d

samsonasik changed the title ~~[TypeDeclaration] Handle fallback from param on ReturnTypeFromReturnNewRector~~ [TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector Sep 17, 2023

samsonasik added 2 commits September 18, 2023 06:10

fix phpstan

ed8860f

fix phpstan

11c285c

staabm reviewed Sep 18, 2023

View reviewed changes

samsonasik added 2 commits September 18, 2023 12:46

realpath() always string after file_exists() on bin/rector.php

402d4ab

Fix phpstan

3bb67a3

staabm reviewed Sep 18, 2023

View reviewed changes

samsonasik added 2 commits September 18, 2023 13:05

cheap check early

8fae378

Trigger CI

a5c966e

samsonasik merged commit 2cf8d83 into main Sep 18, 2023

samsonasik deleted the fallback-param branch September 18, 2023 06:12

TomasVotruba reviewed Sep 18, 2023

View reviewed changes

samsonasik mentioned this pull request Sep 18, 2023

Use realpath() as string on MissingInSetCommand #5040

Merged

	$returnType = $this->returnTypeInferer->inferFunctionLike($node);
	if ($returnType instanceof UnionType) {

Uh oh!

Conversation

samsonasik commented Sep 17, 2023

Uh oh!

samsonasik commented Sep 17, 2023

Uh oh!

samsonasik commented Sep 17, 2023

Uh oh!

samsonasik commented Sep 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samsonasik commented Sep 18, 2023

Uh oh!

samsonasik commented Sep 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samsonasik Sep 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samsonasik commented Sep 18, 2023

Uh oh!

TomasVotruba Sep 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staabm commented Sep 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

samsonasik Sep 18, 2023 •

edited

Loading

TomasVotruba Sep 18, 2023 •

edited

Loading